[eslint] prevent async Promise constructor mistakes #110349
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spalger
added
Team:Operations
…hidden-promise-constructor-rejections
|
Pinging @elastic/kibana-operations (Team:Operations) |
matschaffer
approved these changes
Aug 31, 2021
tylersmalley
approved these changes
Aug 31, 2021
YulNaumenko
approved these changes
Aug 31, 2021
mshustov
approved these changes
Aug 31, 2021
sabarasaba
approved these changes
Aug 31, 2021
darnautov
approved these changes
Aug 31, 2021
thomasneirynck
approved these changes
Aug 31, 2021
JasonStoltz
approved these changes
Aug 31, 2021
…g, fixing manually
…hidden-promise-constructor-rejections
FrankHassanabad
approved these changes
Aug 31, 2021
spalger
added
the
auto-backport
dej611
approved these changes
Aug 31, 2021
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Aug 31, 2021
Co-authored-by: spalger <spalger@users.noreply.github.com>
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
spalger
pushed a commit
to spalger/kibana
that referenced
this pull request
Aug 31, 2021
Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts
spalger
pushed a commit
to spalger/kibana
that referenced
this pull request
Aug 31, 2021
Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # src/plugins/charts/public/services/active_cursor/use_active_cursor.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts # x-pack/plugins/task_manager/server/task_scheduling.ts
spalger
added a commit
that referenced
this pull request
Sep 1, 2021
…110728) * [eslint] prevent async Promise constructor mistakes (#110349) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts * autofix additional violation Co-authored-by: spalger <spalger@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
spalger
added a commit
that referenced
this pull request
Sep 1, 2021
…110731) * [eslint] prevent async Promise constructor mistakes (#110349) Co-authored-by: spalger <spalger@users.noreply.github.com> # Conflicts: # src/plugins/charts/public/services/active_cursor/use_active_cursor.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts # x-pack/plugins/task_manager/server/task_scheduling.ts * update yarn.lock file * add @babel/generator dependency * autofix additional violation * update yarn.lock Co-authored-by: spalger <spalger@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After fixing #109560 I did a little searching and found many cases where we are passing async functions to the Promise constructor without handling the potential of a rejected promise being created by said async function. This PR implements a custom ESLint rule which finds such situations and fixes them by wrapping the body of the function in a
try/catchand routing errors caught toreject().I don't think this produces great code, but it's an improvement over having unhandled promise rejections.
In #109565 I had manually fixed each violation and detailed some strategies for fixing this type of problem for those who are curious, but it felt like I was prescribing a code style in places and felt like we should really prevent this happening in the future with an ESLint rule. Thankfully, my work in #110161 helped me feel more confident in writing fixes like this.